Add hard coded patched for tests #942

danieljvickers · 2025-07-14T03:15:53Z

User description

Description

The primary intent of this PR is to remove hardcoded patches from the test suite, preventing the need to recompile them and saving time during testing. In doing so, I also refactored the geometries because we had no existing support for creating hard-coded patches on any geometries other than line segments, rectangles, and cuboids. Now we can generate hard-coded patches on almost any geometries except for the airfoil geometries (I left a TODO about this) and the model (STL) geometry (this will require it's own dedicated refactor). I also changed every test that used analytic patches to use new hard coded patches, added comments to some code in the toolchain, and updated the docs to note that the old analytic geometry patches are deprecated. I left a print statement warning that those geometries are deprecated, and we should delete them after some time. For now, I had then call subroutines that recreated their original functionality.

~~TODO :: After this PR, we should go back and add the analytic patches as examples that will be skipped so that they are present for external reference.~~

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)
Something else

Scope

This PR comprises a set of related changes with a common goal

If you cannot check the above box, please split your PR into multiple PRs that each have a common goal.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration

This was tested on the full test suite. (./mfc.sh test -j $(nproc))

Test Configuration:

All of my testing was done locally on Ubuntu with the intel and non-intel compilers.

Checklist

I have added comments for the new code
I added Doxygen docstrings to the new code
I have made corresponding changes to the documentation (docs/)
I have added regression tests to the test suite so that people can verify in the future that the feature is behaving as expected
I have added example cases in examples/ that demonstrate my new feature performing as expected.
They run to completion and demonstrate "interesting physics"
I ran ./mfc.sh format before committing my code
New and existing tests pass locally with my changes, including with GPU capability enabled (both NVIDIA hardware with NVHPC compilers and AMD hardware with CRAY compilers) and disabled
This PR does not introduce any repeated code (it follows the DRY principle)
I cannot think of a way to condense this code and reduce any introduced additional line count

If your code changes any code source files (anything in `src/simulation`)

To make sure the code is performing as expected on GPU devices, I have:

Checked that the code compiles using NVHPC compilers
Checked that the code compiles using CRAY compilers
Ran the code on either V100, A100, or H100 GPUs and ensured the new feature performed as expected (the GPU results match the CPU results)
Ran the code on MI200+ GPUs and ensure the new features performed as expected (the GPU results match the CPU results)
Enclosed the new feature via nvtx ranges so that they can be identified in profiles
Ran a Nsight Systems profile using ./mfc.sh run XXXX --gpu -t simulation --nsys, and have attached the output file (.nsys-rep) and plain text results to this PR
Ran a Rocprof Systems profile using ./mfc.sh run XXXX --gpu -t simulation --rsys --hip-trace, and have attached the output file and plain text results to this PR.
Ran my code using various numbers of different GPUs (1, 2, and 8, for example) in parallel and made sure that the results scale similarly to what happens if you run without the new code/feature

PR Type

Enhancement, Tests

Description

Replace analytical patches with hardcoded patches for test optimization
Remove deprecated geometry types 7, 13, and 15
Add hardcoded patch support across all geometry types
Update test cases to use new hardcoded patch system

Changes diagram

flowchart LR
  A["Analytical Patches"] --> B["Hardcoded Patches"]
  C["Geometry Types 7,13,15"] --> D["Deprecated/Removed"]
  E["Test Cases"] --> F["Updated with HCID"]
  G["Patch Processing"] --> H["Enhanced Support"]

Changes walkthrough 📝

Relevant files

Enhancement

5 files

m_patches.fpp `Remove analytical patches and add hardcoded support`	+115/-247
2dHardcodedIC.fpp `Add 2D hardcoded patch cases 280-282`	+31/-1
m_check_patches.fpp `Remove deprecated geometry type checks`	+9/-51
3dHardcodedIC.fpp `Add 3D hardcoded patch case 380`	+10/-0
1dHardcodedIC.fpp `Add 1D hardcoded patch cases 180-181`	+12/-1

Bug fix

1 files

m_checker.fpp `Add variable declaration for loop iterator`	+1/-0

Documentation

1 files

case.py `Add comments to analytical function generation`	+13/-1

Tests

13 files

case.py `Create analytical version of zero circulation vortex`	+109/-0
case.py `Create analytical version of isentropic vortex`	+115/-0
case.py `Create analytical version of acoustic pulse`	+102/-0
case.py `Create analytical version of Taylor-Green vortex`	+100/-0
case.py `Create analytical version of Shu-Osher test`	+74/-0
case.py `Create analytical version of Titarev-Torro test`	+72/-0
cases.py `Skip analytical test cases during testing`	+20/-3
case.py `Update to use hardcoded patch HCID 282`	+3/-4
case.py `Update to use hardcoded patch HCID 380`	+4/-3
case.py `Update to use hardcoded patch HCID 281`	+3/-2
case.py `Update to use hardcoded patch HCID 280`	+5/-4
case.py `Update to use hardcoded patch HCID 180`	+2/-1
case.py `Update to use hardcoded patch HCID 181`	+2/-1

Additional files

14 files

case.md	+7/-6
case.py	+2/-1
case.py	+2/-1
case.py	+2/-1
case.py	+2/-1
case.py	+2/-1
case.py	+1/-1
case.py	+1/-1
case.py	+1/-1
case.py	+1/-1
case.py	+1/-1
case.py	+1/-1
case.py	+1/-1
case.py	+1/-1

Need help?
Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
Check out the documentation for more information.

…t areas of code

…ote the 80s of each case to test values, following the suite of 70s be inheretived value. No particular reason for this other than to keep it away from interesting cases.

… how these patches are added. Will do that soon.

…reading up on patch types

… geometry. There is no way to handle that in the code currently, so I will be adding it in a future commit to resolve the final two examples

…d to be declared

…, adn 15 in a future release

…erences to hard coded patches in the case documentaiton

qodo-merge-pro · 2025-07-14T03:16:33Z

PR Reviewer Guide 🔍

(Review updated until commit `03622a1`)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Duplicate Code The hardcoded patch integration logic is duplicated across multiple geometry functions. Each function has nearly identical code blocks for checking hcid and calling hardcoded macros, which could be refactored into a common helper function. if (patch_icpp(patch_id)%hcid /= dflt_int) then @:Hardcoded1D() end if Magic Number The hardcoded epsilon value `sgm_eps` is used throughout for patch boundary comparisons, replacing the previous hardcoded `1.e-16_wp`. This should be verified to ensure it's properly defined and has the correct value for all use cases. if (1._wp - eta < sgm_eps) patch_id_fp(i, 0, 0) = patch_id Test Coverage The analytical example cases are being skipped in the test suite, which means the new hardcoded patches may not have equivalent test coverage. This could lead to regressions going undetected. "1D_shuosher_analytical", "1D_titarevtorro_analytical", "2D_acoustic_pulse_analytical", "2D_isentropicvortex_analytical", "2D_zero_circ_vortex_analytical", "3D_TaylorGreenVortex_analytical"] if path in casesToSkip:

qodo-merge-pro · 2025-07-14T03:17:24Z

PR Code Suggestions ✨

Latest suggestions up to 03622a1
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Verify patch geometry type selection The geometry value 3 corresponds to a Rectangle patch type according to the documentation table. Verify that this is the intended patch type for the hardcoded initial condition example, as the original value 7 was specifically for hardcoded patches. examples/2D_hardcodied_ic/case.py [58] -"patch_icpp(1)%geometry": 3, +"patch_icpp(1)%geometry": 3, # Rectangle patch - verify this is correct for hardcoded IC Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that the patch geometry has changed from a specific hardcoded type to a standard rectangle, but it only asks for verification of a change that seems intentional given the context of the entire PR.	Low
More

Previous suggestions

✅ Suggestions up to commit 45d40e8

Category	Suggestion	Impact
General	✅ ~~Remove debug print statement~~ Suggestion Impact: The debug print statement was removed from line 5 of the diff, exactly as suggested code diff: - print , "DEBUG: patch_id=", patch_id, " hcid=", patch_icpp(patch_id)%hcid Remove debug print statement from production code. Debug output should not be included in the final codebase as it can clutter logs and impact performance. src/pre_process/include/2dHardcodedIC.fpp [12] -print , "DEBUG: patch_id=", patch_id, " hcid=", patch_icpp(patch_id)%hcid +! Debug print removed Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a debug print statement that should be removed from the final code to avoid cluttering logs and potential performance impacts.	Low

codecov · 2025-07-14T05:32:56Z

Codecov Report

Attention: Patch coverage is 11.11111% with 64 lines in your changes missing coverage. Please review.

Project coverage is 44.07%. Comparing base (293557c) to head (03622a1).

Files with missing lines	Patch %	Lines
src/pre_process/m_patches.fpp	11.59%	47 Missing and 14 partials ⚠️
src/pre_process/m_check_patches.fpp	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #942      +/-   ##
==========================================
+ Coverage   43.76%   44.07%   +0.30%     
==========================================
  Files          68       68              
  Lines       18378    18227     -151     
  Branches     2292     2289       -3     
==========================================
- Hits         8044     8033      -11     
+ Misses       8945     8829     -116     
+ Partials     1389     1365      -24

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

docs/documentation/case.md

src/pre_process/m_patches.fpp

examples/3D_TaylorGreenVortex/case.py

sbryngelson

added several comments. also, some of these hardcoded examples are important. it would be useful to somehow check that you didn't break the case when you switched from analytical to hardcoded.

also probably won't merge this until analytical examples are added (but skipped in the test suite). Presumably, we only need a few of these.

…atements

:

…lchain to skip them so that they do not slow down the test suite.

src/pre_process/m_patches.fpp

Converted these iterators to integers.

sbryngelson · 2025-07-15T20:03:41Z

src/pre_process/m_check_patches.fpp

@@ -74,11 +76,15 @@ contains
                elseif (patch_icpp(i)%geometry == 12) then
                    call s_check_ellipsoid_patch_geometry(i)
                elseif (patch_icpp(i)%geometry == 13) then
-                    call s_check_3D_analytical_patch_geometry(i)
+                    call s_mpi_abort('geometry 13 (formerly "3D Hardcoded")'// &


we have @:prohibit and prohibit_abort for this

sbryngelson · 2025-07-15T20:04:21Z

src/pre_process/m_check_patches.fpp

-                    call s_check_1d_analytical_patch_geometry(i)
+                    call s_mpi_abort('geometry 15 (formerly "1D Hardcoded")'// &
+                                     'is no longer supported for patch '//trim(iStr)// &
+                                     '. Exiting.')
                elseif (patch_icpp(i)%geometry == 16) then
                    print *, '1d pressure sinusoid'


i think we can remove all of these elseifs that just print something. i added them years ago and i don't think they are helping anyone.

sbryngelson · 2025-07-15T20:06:14Z

src/pre_process/m_patches.fpp


    end subroutine s_circle

+    ! TODO :: Determine if we want analytical and hardcoded patches for the airfoil geometries


we don't, can remove this

sbryngelson · 2025-07-15T20:09:26Z

This seems fine to me, pending removing some comments / extra junk. Also, if @wilfonba approves, since he uses a lot of these features and added some of them to the code to begin with.

wilfonba · 2025-07-15T23:39:04Z

docs/documentation/case.md

@@ -313,7 +314,7 @@ These parameters should be prepended with `patch_ib(j)%` where $j$ is the patch
 #### Parameter Descriptions

 - `geometry` defines the type of geometry of a patch with an integer number.
-Definitions for currently implemented patch types are list in table [Immersed Boundary Patch Type](#immersed-boundary-patch-types)
+Definitions for currently implemented patch types are list in table [Patch Types](#patch-types).


This should be reverted unless I'm missing something. The "immersed boundary patch types" table appears to still exist.

wilfonba · 2025-07-15T23:42:04Z

src/pre_process/include/2dHardcodedIC.fpp

+            q_prim_vf(E_idx)%sf(i, j, 0) = 1.0*(1.0 - (1.0/1.0)*(5.0/(2.0*3.141592653589793))*(5.0/(8.0*1.0*(1.4 + 1.0)*3.141592653589793))*exp(2.0*1.0*(1.0 - (x_cc(i) - patch_icpp(1)%x_centroid)**2.0 - (y_cc(j) - patch_icpp(1)%y_centroid)**2.0)))**(1.4 + 1.0)
+            q_prim_vf(contxb + 0)%sf(i, j, 0) = 1.0*(1.0 - (1.0/1.0)*(5.0/(2.0*3.141592653589793))*(5.0/(8.0*1.0*(1.4 + 1.0)*3.141592653589793))*exp(2.0*1.0*(1.0 - (x_cc(i) - patch_icpp(1)%x_centroid)**2.0 - (y_cc(j) - patch_icpp(1)%y_centroid)**2.0)))**1.4
+            q_prim_vf(momxb + 0)%sf(i, j, 0) = 0.0 + (y_cc(j) - patch_icpp(1)%y_centroid)*(5.0/(2.0*3.141592653589793))*exp(1.0*(1.0 - (x_cc(i) - patch_icpp(1)%x_centroid)**2.0 - (y_cc(j) - patch_icpp(1)%y_centroid)**2.0))
+            q_prim_vf(momxb + 1)%sf(i, j, 0) = 0.0 - (x_cc(i) - patch_icpp(1)%x_centroid)*(5.0/(2.0*3.141592653589793))*exp(1.0*(1.0 - (x_cc(i) - patch_icpp(1)%x_centroid)**2.0 - (y_cc(j) - patch_icpp(1)%y_centroid)**2.0))


There's a constant for pi in m_constants that will clean these lines up a lot

wilfonba · 2025-07-15T23:44:16Z

src/pre_process/include/1dHardcodedIC.fpp

+    case (181)
+        ! This is patch is hard-coded for test suite optimization used in the
+        ! 1D_titarevtorro cases: "patch_icpp(2)%alpha_rho(1)": "1 + 0.1*sin(20*x*pi)"
+        q_prim_vf(contxb + 0)%sf(i, 0, 0) = 1 + 0.1*sin(20*x_cc(i)*3.141592653589793)


There's a constant for 'pi' in 'm_constants' that will clean these lines up a lot

wilfonba · 2025-07-15T23:46:48Z

I added some minor comments. I don't see any other issues with these changes

danieljvickers added 21 commits July 8, 2025 12:51

Added comments after digging through the toolchain to find the correc…

bace49c

…t areas of code

Successfully added a single patch to the preprocessor. Decided to den…

5c86cf7

…ote the 80s of each case to test values, following the suite of 70s be inheretived value. No particular reason for this other than to keep it away from interesting cases.

Actually fixed error. Need some better documentation to make it clear…

36b147c

… how these patches are added. Will do that soon.

Some small changes to the documentation to clarify issues I saw when …

17d0442

…reading up on patch types

All 1D example cases now work

2690009

Added another case

f76e8cf

Added the 2D_isentropic case:

0544597

Added the 2D_zero_circ_vortex case

523f103

Added two more cases

54a21c1

Fixed errors with the shousher cases

74accd8

More resolved test cases

48143e9

Almost all patches work except for patches that require a 2D ciruclar…

97a0ad8

… geometry. There is no way to handle that in the code currently, so I will be adding it in a future commit to resolve the final two examples

First patch has the rework so that different HC geometries do not nee…

409ce90

…d to be declared

Changes all of the shoucher cases to use the new geometry system

425ca0e

Updated all 1D cases

d897574

Everything is compiling

28e7312

Added deprication statements. We should remove patch geometries 7, 13…

26c112c

…, adn 15 in a future release

Everything works except for the 2D_Zero_Circ_vortex case. Removed ref…

380721d

…erences to hard coded patches in the case documentaiton

Forgot to add a separate case for the 2D cases. Everything works now

a3273b7

Merge branch 'MFlowCode:master' into add-hard-coded-patched-for-tests

9a80857

Merged in upstream and formatted

45d40e8

danieljvickers requested review from a team and sbryngelson as code owners July 14, 2025 03:15

qodo-merge-pro bot added the Review effort 4/5 label Jul 14, 2025

Did not intend to push this yaml file

1e76121

danieljvickers added 2 commits July 13, 2025 23:28

Removed double precision intrinsics in HCID 280

62056ce

Fixed spelling errors

9b09313